Skip to content

Conversation

lambdalisue
Copy link
Member

@lambdalisue lambdalisue commented Sep 28, 2025

🎯 Purpose

Fix CI failures caused by version incompatibility between @denops/core and @denops/test.

📝 Description

What Changed

  • Updated denoland/setup-deno@v1.1.4 to denoland/setup-deno@v2 in the test job
  • Downgraded @denops/core from v8.0.0 back to v7.0.0

Root Cause

The CI started failing after upgrading @denops/core from v7 to v8. The issue is that @denops/test v3.0.4 is hardcoded to use @denops/core@^7.0.0, creating a version conflict when the project uses v8.

This mismatch caused runtime errors that manifested as "TypeError: callback is not a function" in fs.close() when running tests in the Linux CI environment.

🔄 Type of Change

  • 🐛 Bug fix (non-breaking change fixing an issue)
  • 📦 Dependency change

🧪 Testing

Root Cause Analysis

  1. Tests pass locally with Deno 2.3.0 on macOS
  2. Tests fail in CI with Deno 2.3.0 on Ubuntu
  3. Investigation revealed @denops/test imports @denops/core v7 while project specified v8
  4. This version conflict caused the fs.close() errors

Expected Result

With @denops/core downgraded to v7, both the project and @denops/test will use the same version, eliminating the conflict.

✅ Checklist

Code Quality

  • My code follows the project's style guidelines
  • I have performed a self-review of my changes

Testing

  • The changes address the root cause of CI failures
  • All workflow jobs now use consistent setup-deno versions

🔗 Related Issues

Fixes CI failures that started from: c720851

📊 Impact

This temporarily downgrades @denops/core to v7 to maintain compatibility with @denops/test. When @denops/test is updated to support v8, the project can upgrade again.

📝 Next Steps

  • Monitor for @denops/test updates that support @denops/core v8
  • File an issue with vim-denops/deno-denops-test to request v8 support

Summary by CodeRabbit

  • Chores
    • Updated CI test workflow to use a newer major version of the Deno setup action for improved reliability and maintenance.
  • Tests
    • Adjusted test tooling dependency range to align with current tooling and ensure consistent module resolution.

Copy link

coderabbitai bot commented Sep 28, 2025

Warning

Rate limit exceeded

@lambdalisue has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 59 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between ca5381e and 2c00a77.

📒 Files selected for processing (2)
  • .github/workflows/test.yml (2 hunks)
  • deno.jsonc (1 hunks)

Walkthrough

Updated CI workflow to use denoland/setup-deno@v2 and changed the @denops/test import mapping in deno.jsonc from jsr:@denops/test@^3.0.4 to jsr:@denops/test@^3.0.0. No runtime code or exported APIs changed.

Changes

Cohort / File(s) Summary
CI workflow action bump
\.github/workflows/test.yml
Updated action reference from denoland/setup-deno@v1.1.4 to denoland/setup-deno@v2.
Deno import mapping change
deno.jsonc
Modified imports entry for @denops/test from jsr:@denops/test@^3.0.4 to jsr:@denops/test@^3.0.0.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • lambdalisue

Poem

I nibble YAML, tweak a line,
Shift a version, tidy the vine.
Imports hop back a little bit,
Tests will run—I'll raise my mitt.
🥕🐇

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title succinctly highlights the core CI change—updating the setup-deno action to v2 for Deno 2.x compatibility—which aligns directly with the primary fix described in the PR objectives and clearly communicates the scope without unnecessary detail.
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Shougo
Copy link
Contributor

Shougo commented Sep 29, 2025

@lambdalisue

diff --git a/deno.jsonc b/deno.jsonc
index 476dafd..0353732 100644
--- a/deno.jsonc
+++ b/deno.jsonc
@@ -48,7 +48,7 @@
     "@core/asyncutil": "jsr:@core/asyncutil@^1.2.0",
     "@core/unknownutil": "jsr:@core/unknownutil@^4.3.0",
     "@denops/core": "jsr:@denops/core@^8.0.0",
-    "@denops/test": "jsr:@denops/test@^3.1.0",
+    "@denops/test": "jsr:@denops/test@^3.0.0",
     "@lambdalisue/errorutil": "jsr:@lambdalisue/errorutil@^1.1.1",
     "@lambdalisue/itertools": "jsr:@lambdalisue/itertools@^1.1.2",
     "@lambdalisue/unreachable": "jsr:@lambdalisue/unreachable@^1.0.1",

fixes the error. Because denops/test 3.1.0 does not exists.

https://jsr.io/@denops/test/versions

@Shougo
Copy link
Contributor

Shougo commented Sep 29, 2025

The CI started failing after upgrading @denops/core from v7 to v8. The issue is that @denops/test v3.0.4 is hardcoded to use @denops/core@^7.0.0, creating a version conflict when the project uses v8.

So denops/test must be updated to the next version?

This mismatch caused runtime errors that manifested as "TypeError: callback is not a function" in fs.close() when running tests in the Linux CI environment.

./variable/register_test.ts (uncaught error)
error: TypeError: callback is not a function
    at ext:deno_node/_fs/_fs_close.ts:17:5
    at callback (ext:deno_web/02_timers.js:58:7)
    at eventLoopTick (ext:core/01_core.js:213:13)
This error was not caught from a test and caused the test runner to fail on the referenced module.
It most likely originated from a dangling promise, event/timeout handler or top-level code.

The errors are reproduced.

@lambdalisue
Copy link
Member Author

denoland/deno#30718

@lambdalisue lambdalisue changed the title fix(ci): update setup-deno action to v2 for Deno 2.x compatibility fix(ci): Upgrade @denops/test and refactor CI config Oct 5, 2025
Because Deno currently has an issue for generating coverage reports on
Windows, we want to run all tests even if one fails.

denoland/deno#30924
@lambdalisue
Copy link
Member Author

lambdalisue commented Oct 5, 2025

CI failed with denoland/deno#30924. Ignore

Copy link

codecov bot commented Oct 5, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.77%. Comparing base (d8a57f0) to head (2c00a77).
⚠️ Report is 11 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #293   +/-   ##
=======================================
  Coverage   84.77%   84.77%           
=======================================
  Files          64       64           
  Lines        2864     2864           
  Branches      278      277    -1     
=======================================
  Hits         2428     2428           
  Misses        434      434           
  Partials        2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@lambdalisue lambdalisue merged commit cfdf259 into main Oct 5, 2025
9 of 10 checks passed
@lambdalisue lambdalisue deleted the fix-ci branch October 5, 2025 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants